Skip to content

Conversation

@grahamking
Copy link
Contributor

@grahamking grahamking commented Oct 9, 2025

Since #3350 the MDC is attached to an etcd lease, so it cleans up on shutdown. We don't need to manually clear namespace any more.

If there are any remaining non-lease keys (I'm not aware of any, and I have looked), we will treat that as a bug.

Summary by CodeRabbit

  • New Features
    • Faster startup: launch scripts no longer perform pre-launch namespace clearing across sglang and TensorRT-LLM backends.
  • Chores
    • Simplified aggregated/disaggregated launch flows by removing the namespace-clear step while keeping service orchestration unchanged.
  • Documentation
    • Updated backend manual to remove the clear step and reflect the streamlined startup process.
  • Tests
    • Adjusted test launch sequence to align with the new startup behavior.

Since #3350 the MDC is attached
to an etcd lease, so it cleans up on shutdown. We don't need to manually
clear namespace any more.

If there are any remaining non-lease keys (I'm not aware of any, and I
have looked), we will treat that as a bug.

Signed-off-by: Graham King <[email protected]>
@grahamking grahamking requested review from a team as code owners October 9, 2025 13:12
@github-actions github-actions bot added the chore label Oct 9, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 9, 2025

Walkthrough

Removed the clear_namespace utility and all its invocations. Launch scripts for sglang and trtllm now start ingress/frontend and workers without a pre-clean step. The Python CLI modules, Rust runtime method, and Python bindings exposing temp_clear_namespace were deleted. Documentation and a test script were updated accordingly.

Changes

Cohort / File(s) Summary
SGLang launch scripts
components/backends/sglang/launch/agg.sh, .../agg_embed.sh, .../agg_router.sh, .../disagg.sh, .../disagg_dp_attn.sh, .../multimodal_agg.sh, .../multimodal_disagg.sh
Removed calls to python3 -m dynamo.sglang.clear_namespace; startup flows now directly launch ingress/frontend and workers.
TRT-LLM launch scripts
components/backends/trtllm/launch/agg.sh, .../agg_metrics.sh, .../agg_router.sh, .../disagg.sh, .../disagg_router.sh, .../epd_disagg.sh, .../gpt_oss_disagg.sh
Deleted python3 utils/clear_namespace.py invocations; scripts now start frontend/workers without namespace clearing.
Clear-namespace utilities removed (Python)
components/backends/trtllm/utils/clear_namespace.py, components/src/dynamo/sglang/clear_namespace.py
Removed modules providing CLI and async clear_namespace functions.
Runtime API removals (Rust + Python bindings)
lib/runtime/src/distributed.rs, lib/bindings/python/rust/lib.rs
Deleted DistributedRuntime::temp_clear_namespace and its Python binding method.
Docs
components/backends/trtllm/gpt-oss.md
Updated manual launch instructions: removed “Clear namespace” step; renamed step to “Start frontend”.
Tests
tests/serve/launch/sglang_agg.sh
Removed clear_namespace call from test launch script.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Op as Operator
  participant L as Launch Script
  participant F as Frontend/Ingress
  participant W as Worker(s)

  rect rgba(230,245,255,0.5)
  note over L: Previous flow
  Op->>L: Run script
  L->>L: Clear namespace (removed)
  L->>F: Start frontend/ingress
  L->>W: Start worker(s)
  end

  rect rgba(235,255,235,0.5)
  note over L: New flow
  Op->>L: Run script
  L--xL: Clear namespace (skipped)
  L->>F: Start frontend/ingress
  L->>W: Start worker(s)
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I hop through scripts, light on my feet,
Swept the brooms away—so clean, so neat.
No more clearing paths before we go,
We start the show with a nimble flow.
Thump-thump! 🥕 The queues advance—
Fewer steps, same rabbit dance.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description does not use the required template headings; it omits the Overview, Details, Where should the reviewer start, and Related Issues sections specified in the repository’s description template. Please structure the PR description to include the required sections: an Overview describing the change, a Details section outlining the modifications, a Where should the reviewer start section listing key files, and a Related Issues section referencing any linked issues.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately describes the main change—removal of the clear_namespace script—and is concise and clear.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f712653 and cb93954.

📒 Files selected for processing (20)
  • components/backends/sglang/launch/agg.sh (0 hunks)
  • components/backends/sglang/launch/agg_embed.sh (0 hunks)
  • components/backends/sglang/launch/agg_router.sh (0 hunks)
  • components/backends/sglang/launch/disagg.sh (0 hunks)
  • components/backends/sglang/launch/disagg_dp_attn.sh (0 hunks)
  • components/backends/sglang/launch/multimodal_agg.sh (0 hunks)
  • components/backends/sglang/launch/multimodal_disagg.sh (0 hunks)
  • components/backends/trtllm/gpt-oss.md (1 hunks)
  • components/backends/trtllm/launch/agg.sh (0 hunks)
  • components/backends/trtllm/launch/agg_metrics.sh (0 hunks)
  • components/backends/trtllm/launch/agg_router.sh (0 hunks)
  • components/backends/trtllm/launch/disagg.sh (0 hunks)
  • components/backends/trtllm/launch/disagg_router.sh (0 hunks)
  • components/backends/trtllm/launch/epd_disagg.sh (0 hunks)
  • components/backends/trtllm/launch/gpt_oss_disagg.sh (0 hunks)
  • components/backends/trtllm/utils/clear_namespace.py (0 hunks)
  • components/src/dynamo/sglang/clear_namespace.py (0 hunks)
  • lib/bindings/python/rust/lib.rs (0 hunks)
  • lib/runtime/src/distributed.rs (0 hunks)
  • tests/serve/launch/sglang_agg.sh (0 hunks)
💤 Files with no reviewable changes (19)
  • components/backends/trtllm/launch/disagg_router.sh
  • components/backends/sglang/launch/multimodal_agg.sh
  • components/backends/trtllm/launch/gpt_oss_disagg.sh
  • components/backends/trtllm/launch/disagg.sh
  • components/backends/sglang/launch/disagg.sh
  • components/backends/sglang/launch/multimodal_disagg.sh
  • components/backends/trtllm/utils/clear_namespace.py
  • components/backends/trtllm/launch/agg_metrics.sh
  • components/src/dynamo/sglang/clear_namespace.py
  • tests/serve/launch/sglang_agg.sh
  • lib/bindings/python/rust/lib.rs
  • components/backends/sglang/launch/agg_embed.sh
  • components/backends/trtllm/launch/agg.sh
  • lib/runtime/src/distributed.rs
  • components/backends/trtllm/launch/epd_disagg.sh
  • components/backends/sglang/launch/agg.sh
  • components/backends/sglang/launch/disagg_dp_attn.sh
  • components/backends/sglang/launch/agg_router.sh
  • components/backends/trtllm/launch/agg_router.sh
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: tests (.)
  • GitHub Check: tests (launch/dynamo-run)
  • GitHub Check: tests (lib/bindings/python)
  • GitHub Check: tests (lib/runtime/examples)
  • GitHub Check: clippy (.)
  • GitHub Check: clippy (lib/bindings/python)
  • GitHub Check: clippy (launch/dynamo-run)
  • GitHub Check: clippy (lib/runtime/examples)
  • GitHub Check: Build and Test - dynamo

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@grahamking grahamking merged commit e9a7100 into main Oct 9, 2025
28 checks passed
@grahamking grahamking deleted the gk-clear-namespace-tidy branch October 9, 2025 17:44
ziqifan617 pushed a commit that referenced this pull request Oct 20, 2025
nv-tusharma pushed a commit that referenced this pull request Oct 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants